Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge cov-core and pytest-cov packages and other fixes #58

Merged
merged 47 commits into from
Jul 6, 2015
Merged

Merge cov-core and pytest-cov packages and other fixes #58

merged 47 commits into from
Jul 6, 2015

Conversation

ionelmc
Copy link
Member

@ionelmc ionelmc commented Jun 30, 2015

First, I'm terribly sorry to make a PR like this, I got a bit impatient and there wasn't any activity on pytest-cov for a while so I made a fork. This PR includes the changes from that fork. Please don't send me back to the loony bin 😁

Changes I did and some of the goals behind them:

  • Keep the same arguments as for pytest-cov 1.x. No breaking changes, but still bring in the fixes regarding .coveragerc handling:
    • Using bare --cov will use the sources specified in .coveragerc. Handling here. This means the --cov-source option from the 2.0 branch is gone.
    • Using --cov-report=term will report missing lines if .coveragerc has show_missing = True
    • The option --cov-fail-under is automatically activated if .coveragerc has fail_under = ... (only if using coverage 4.0)
    • The option --cov-min is renamed to --cov-fail-under to keep consistency with coverage docs (so that users will have an easier time finding the option).
  • Fix the .pth activation. I found a solution that works when installing from install, easy_install, wheels and develop. Uninstall also works, but only for wheels.
  • Some improvements for CI (separate testing with xdist and windows CI via AppVeyor)
  • Merging the cov-core and pytest-cov packages to simplify things like the release process or compatibility problems caused by mismatched versions. I took an opinionated turn here I know but there's value in focusing on a single test tool.

Todo

  • classifiers
  • update setup.py (remove "Forked from pytest-cov", maybe other fixes)
  • update LICENSE (re-add me as author, you can leave yours)
  • update changelog
  • update Acknowledgements in README
  • update release date (don't know it yet)
  • sort out command line options

@schlamar
Copy link
Contributor

Using bare --cov will use the sources specified in .coveragerc. Handling here. This means the --cov-source option from the 2.0 branch is gone.

The reason for deprecating --cov with parameters is that per convention any command line parameter should have either arguments or no argument at all.

An alternative would be to keep the behavior as it is and introduce a new parameter - e.g. --covrc - which activates coverage and reads sources from .coveragerc.

Or another option would be to always activate the plugin if there is a .coveragerc file and provide an option to disable it, something like --cov-disable.

@schlamar
Copy link
Contributor

Thanks for contributing, BTW. Looks very great in general 🍰

@schlamar
Copy link
Contributor

Is there anything left from the todo list in #28 (minus the points which are obviously obsolete)?

@ionelmc
Copy link
Member Author

ionelmc commented Jun 30, 2015

The reason for deprecating --cov with parameters is that per convention any command line parameter should have either arguments or no argument at all. Another option would be to keep the behavior as it is and introduce a new parameter - e.g. --covrc - which activates coverage and reads sources from .coveragerc.

Or another option would be to always activate the plugin if there is a .coveragerc file and provide an option to disable it, something like --cov-disable.

I agree it's not the cleanest cmdline api. Had pytest-cov not been already released with this --cov bi-valence I'd agree 100% with you. However, it's already released - are we getting any serious advantages by making users change their commands?

@ionelmc
Copy link
Member Author

ionelmc commented Jun 30, 2015

Is there anything left from the todo list in #28 (minus the points which are obviously obsolete).

I think this it's missing:

  • update classifiers in setup.py

The rest should be fixed.

We might need to flatten out a bit the current changelog.

@schlamar
Copy link
Contributor

Had pytest-cov not been already released with this --cov bi-valence I'd agree 100% with you.

It's not released yet, only pending in the 2.0 branch ;)

@schlamar schlamar mentioned this pull request Jun 30, 2015
12 tasks
@schlamar
Copy link
Contributor

update classifiers in setup.py

I guess we can use the classifiers from https://github.com/ionelmc/pytest-cover/blob/master/setup.py ?

@ionelmc
Copy link
Member Author

ionelmc commented Jun 30, 2015

It's not released yet, only pending in the 2.0 branch ;)

You are right. But what if we just add a --cov-enable option? (to avoid deprecating --cov=foobar). Hmmm, that's not pretty either ...

Do people want the --cov-source?

@ionelmc
Copy link
Member Author

ionelmc commented Jun 30, 2015

I guess we can use the classifiers from https://github.com/ionelmc/pytest-cover/blob/master/setup.py ?

Then it's fixed :)

@schlamar
Copy link
Contributor

Or another option would be to always activate the plugin if there is a .coveragerc file and provide an option to disable it, something like --cov-disable.

I guess this is my favorite right now. Any thoughts or other ideas?

@ionelmc
Copy link
Member Author

ionelmc commented Jun 30, 2015

I guess this is my favorite right now. Any thoughts or other ideas?

I suspect explicit opt-out would annoy some people.

What disadvantages do we have with the dual --cov / --cov=path arguments?

@schlamar
Copy link
Contributor

schlamar commented Jul 3, 2015

What disadvantages do we have with the dual --cov / --cov=path arguments?

I'll still think about it. :) But I guess the alternatives are not really better so we can leave it like this.

I've added some minor points as todo list to the first post. But in general it looks really great. I think we are almost good to go and can release this soon :)

@ionelmc
Copy link
Member Author

ionelmc commented Jul 4, 2015

Alright, I've fixed those and squashed a little bit the unforking changes.

@schlamar
Copy link
Contributor

schlamar commented Jul 6, 2015

One last small thing: Rename tests/test_pytest_cover.py back :)

Then I'll merge everything back in master and have a last look before release.

Thank you for this great PR 🍰

@schlamar
Copy link
Contributor

schlamar commented Jul 6, 2015

I'll do this on my own, merging it now.

schlamar added a commit that referenced this pull request Jul 6, 2015
Merge cov-core and pytest-cov packages and other fixes
@schlamar schlamar merged commit e03031d into pytest-dev:2.0 Jul 6, 2015
@ionelmc
Copy link
Member Author

ionelmc commented Jul 6, 2015

Ah yes, I'm a bit slow today.

About releasing, this is what I usually do:

rm -rf dist build src/*.egg-info; python3.4 setup.py clean --all sdist && twine upload dist/*

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants